[8.x](backport #41755) [AWS] [S3] fix: improve object size metric calculation#41778
Merged
Kavindu-Dodan merged 3 commits into8.xfrom Nov 25, 2024
Merged
[8.x](backport #41755) [AWS] [S3] fix: improve object size metric calculation#41778Kavindu-Dodan merged 3 commits into8.xfrom
Kavindu-Dodan merged 3 commits into8.xfrom
Conversation
* rely on monitored reader for content length Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> * add tests to validate metrics Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> * add changelog entry Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> * fix lint - ignore ok values as we know the stored value type Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> * review change - rename and accessor Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> --------- Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> (cherry picked from commit 92bb2c5)
botelastic
bot
added
the
needs_team
Kavindu-Dodan
added
the
Team:obs-ds-hosted-services
Contributor
|
Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services) |
botelastic
bot
removed
the
needs_team
Kavindu-Dodan
approved these changes
Nov 25, 2024
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed commit message
This fixes a bug with Cloudflare logpush integration, which uses the S3 input.
#40775 introduced a feature introducing s3_object_size_in_bytes histogram. Internally it utilized S3 SDK's GetObjectOutput instances' ContentLength property 1. In code level (SDK internally), this is derived using Content-Length header 2.
It seems that Cloudflare R2 (API compatible with S3) could omit Content-Length header 3. Hence, I have improved how we derive the
s3_object_size_in_bytesmetric by avoiding using Content-Length backed method.Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.How to test this PR locally
Screenshots
See metric comparison with proposed collection method (before removing content length usage). Tracked total bytes are same as content length.
Handling validated for gzip
Handling validated for plain text
This is an automatic backport of pull request #41755 done by [Mergify](https://mergify.com).
Footnotes
https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html#API_GetObject_ResponseSyntax ↩
https://github.com/aws/aws-sdk-go-v2/blob/service/s3/v1.60.1/service/s3/deserializers.go#L5477-L5484 ↩
https://community.cloudflare.com/t/no-content-length-header-when-serving-plaintext-from-r2/565521 ↩